-
Notifications
You must be signed in to change notification settings - Fork 13.6k
rustdoc template font links only emit crossorigin
when needed
#144467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
rustdoc template font links only emit crossorigin
when needed
#144467
Conversation
rustbot has assigned @GuillaumeGomez. Use |
Seems ok to. Do you see any potential issue @notriddle? |
7db45e4
to
24eb433
Compare
crossorigin
when neededcrossorigin
when needed
Updated check should be a lot more robust, and adjusted naming/comments to discourage other [mis]usage |
This comment was marked as outdated.
This comment was marked as outdated.
f462760
to
f10e91e
Compare
Alright I have made it even more robust based on https://url.spec.whatwg.org/#url-parsing scheme parsing 😅 |
@GuillaumeGomez Can you take another look? Should properly handle all schemes based on the spec now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You removed the tests. ^^'
The `crossorigin` attribute may cause issues when the href is not actuall across origins. Specifically, the tag causes the browser to send a preflight OPTIONS request to the href even if it is same-origin. Some tempermental servers may reject all CORS preflect requests even if they're actually same-origin, which causes a CORS error and prevents the fonts from loading, even later on. This commit fixes that problem by not emitting `crossorigin` if the url looks like a domain-relative url. Co-authored-by: Guillaume Gomez <[email protected]>
89df295
to
f516d4c
Compare
Alright I have added the unit tests (though I am unsure how to run them locally) |
You can run unit tests in rustdoc like this: |
Just one last nit and we're there. :) |
f516d4c
to
340984b
Compare
This comment has been minimized.
This comment has been minimized.
Hmm 😅 |
Arf, sorry about that. I definitely don't like this approach but if it's the one enforced in the project... |
340984b
to
f516d4c
Compare
@bors r+ rollup |
…c-fix-cors, r=GuillaumeGomez rustdoc template font links only emit `crossorigin` when needed The `crossorigin` attribute may cause issues when the href is not actually cross-origin. Specifically, the tag causes the browser to send a preflight OPTIONS request to the server even if it is same-origin. Some temperamental servers may reject all CORS preflight requests even if they're actually same-origin, which causes a CORS error and prevents the fonts from loading, even later on. This commit fixes that problem by not emitting `crossorigin` if the url appears to be relative to the same origin.
The
crossorigin
attribute may cause issues when the href is not actually cross-origin. Specifically, the tag causes the browser to send a preflight OPTIONS request to the server even if it is same-origin. Some temperamental servers may reject all CORS preflight requests even if they're actually same-origin, which causes a CORS error and prevents the fonts from loading, even later on.This commit fixes that problem by not emitting
crossorigin
if the url appears to be relative to the same origin.